Skip to content

Conversation

@aeris
Copy link
Contributor

@aeris aeris commented Sep 6, 2025

Lumberjack is unmaintained since 2 years.
A fork exists, Timberjack to replace it with more features.

This PR migrate to Timberjack and add support newest features, specially RotateAt options to allow to rotate log file base on date.

Assistance Disclosure

No AI was used.

@CLAassistant
Copy link

CLAassistant commented Sep 6, 2025

CLA assistant check
All committers have signed the CLA.

@francislavoie
Copy link
Member

Oh, fantastic! I didn't get the memo that a fork was worked on, we were waiting for someone to pick up the mantle. Will review soon.

@francislavoie francislavoie changed the title Move to timberjack and add rotate-at methods logging: Switch from lumberjack to timberjack add time-rolling options Sep 7, 2025
@francislavoie francislavoie changed the title logging: Switch from lumberjack to timberjack add time-rolling options logging: Switch from lumberjack to timberjack, add time-rolling options Sep 7, 2025
@francislavoie francislavoie added this to the v2.11.0 milestone Sep 14, 2025
@mholt
Copy link
Member

mholt commented Sep 17, 2025

This is looking great. I am happy to merge this, I just need to give it one closer look first.

Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok yeah this looks great. Thank you!

I second Francis' comments -- if we can clarify some docs and address the things he brought up, I'd say let's merge it. 👍

@aeris
Copy link
Contributor Author

aeris commented Sep 18, 2025

👍 I try to fix docs & cie this week-end !

@francislavoie
Copy link
Member

Thanks!

FYI timberjack had a new release just now to add support for zstd compression (we should expose that option too) & adding a reason to manual rotation (so we could add support for rotating the files on config reload).

Another requested nice-to-have, we could also add support for opt-in reopening of the log file on config reload, useful for when users use their own external log rolling instead of what we provide them, we should reopen the log file so it correctly points to the new file instead of the moved file.

Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good to me now. @francislavoie What do you think? (Maybe we can add the zstd and reopening in a later PR if not this one.)

@aeris
Copy link
Contributor Author

aeris commented Oct 3, 2025

I can try another PR for the zstd support

@aeris aeris force-pushed the timberjack-rotate-at branch 2 times, most recently from 0999b6d to 1a41234 Compare October 11, 2025 08:06
@francislavoie
Copy link
Member

francislavoie commented Oct 11, 2025

Huh, we got a data race, that's not good. @DeRuina maybe you have an idea what the problem is?

@DeRuina
Copy link

DeRuina commented Oct 13, 2025

@francislavoie I'm fixing the issues

Copy link

@DeRuina DeRuina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timberjack has a new version v1.3.8 which was just published. All of the data races issues are solved.

You can proceed and merge the PR

@DeRuina
Copy link

DeRuina commented Oct 15, 2025

@francislavoie new version available

@mholt
Copy link
Member

mholt commented Oct 15, 2025

Thank you @DeRuina !

@aeris How would you feel about updating the dependency? Then we can merge this.

aeris added a commit to aeris/caddy that referenced this pull request Oct 15, 2025
@aeris
Copy link
Contributor Author

aeris commented Oct 15, 2025

Upgraded and pushed 😊

@francislavoie
Copy link
Member

I think you also need to run go mod tidy to fix up the go.sum

@francislavoie
Copy link
Member

I went ahead and ran go mod tidy and pushed a commit, let's see if CI passes now

@francislavoie francislavoie requested a review from DeRuina October 15, 2025 21:04
@francislavoie francislavoie removed the request for review from DeRuina October 15, 2025 21:04
@francislavoie francislavoie merged commit 10ac7da into caddyserver:master Oct 15, 2025
23 checks passed
@francislavoie
Copy link
Member

Thanks everyone!

aeris added a commit to aeris/caddy that referenced this pull request Oct 15, 2025
@aeris
Copy link
Contributor Author

aeris commented Oct 15, 2025

\o/

@DeRuina
Copy link

DeRuina commented Oct 16, 2025

Happy this was merged. If you guys need any other features in the future like adding more compression options or anything else that will serve caddy let me know and I will work on it :)

@mholt
Copy link
Member

mholt commented Oct 22, 2025

Awesome, thank you @DeRuina ! We'd be happy to collaborate with you on more contributions. (Plenty of open issues -- and PRs that could use reviews. We're looking to grow the team.)

@DeRuina
Copy link

DeRuina commented Oct 23, 2025

@mholt I would be interested and would like to hear more!

@francislavoie
Copy link
Member

@DeRuina Matt is saying, if you'd like to go look through open Caddy issues for something that looks interesting for you to help with, that would be a good way to start if you'd like to help out with something 😅

@mholt
Copy link
Member

mholt commented Oct 23, 2025

Yep. Just get involved with fixing bugs and addressing issues, reviewing existing PRs to help us know whether to merge them, etc, and it may not be long before we add you as a collaborator :)

@mohammed90 mohammed90 mentioned this pull request Oct 25, 2025
46 tasks
@DeRuina
Copy link

DeRuina commented Nov 3, 2025

No problem I created #7335 that solves #7314 and I will looking for some open PR's I could review :)

@github-actions github-actions bot mentioned this pull request Dec 3, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants